Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always set index #828

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Always set index #828

merged 1 commit into from
Oct 15, 2024

Conversation

newzealandpaul
Copy link
Contributor

The index should always be set, because it will be accessed later in the script and generate a runtime error

What does this PR do?

Sets the index variable to the config's id_start_number, even if id_field was already in the header

Why is this desirable ?

When each row is iterated later on in the script, index will be accessed. If it is not set, a runtime error occurs.

        if config["id_field"] in headers:
            row[config["id_field"]] = index + reader.line_num - 2

The index should always be set, because it will be accessed later in the script and generate a runtime error
@mjordan
Copy link
Owner

mjordan commented Oct 3, 2024

Thanks @newzealandpaul . How would I test this change?

@newzealandpaul
Copy link
Contributor Author

newzealandpaul commented Oct 3, 2024 via email

@newzealandpaul
Copy link
Contributor Author

If index is not set this error occurs, which makes sense:

Traceback (most recent call last):
 File "islandora_workbench/i7Import/get_islandora_7_content.py", line 118, in <module>
   row[config["id_field"]] = index + reader.line_num - 2
                             ^^^^^
NameError: name 'index' is not defined

My condig looks like this:

solr_base_url: 'http://X/solr/collection'
islandora_base_url: 'http://X/'
csv_output_path: 'islandora7_metadata.csv'
obj_directory: '/tmp/objs'
log_file_path: 'islandora_content.log'
id_field: 'PID'
rows: 100
id_start_number: 1
pids_to_skip: []
collection: islandora:root
debug: true
deep_debug: true

I think this is a valid config, with my install the pull request proposed here is required. I think this would effect other users too . There can't be any downside to setting the index.

@mjordan mjordan merged commit 44a873d into mjordan:main Oct 15, 2024
5 checks passed
@mjordan
Copy link
Owner

mjordan commented Oct 15, 2024

OK, thank you, merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants